-
Notifications
You must be signed in to change notification settings - Fork 745
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SQL Script fix for Portal Information #2522
SQL Script fix for Portal Information #2522
Conversation
Fixes #2521 by rebuilding the PortalsDefaultLanguage view
Fix for missing SQL change
@dnnsoftware/tag I believe this is the right way to fix this, it is a breaking change though for anyone that has already tested a 9.3.0 upgrade, but I believe that would be the case no matter what. If I need to change let me know |
I doubt that this is the proper fix. it looks like a data call in code behind is not matching columns properly, SQL is always matching by column names. |
Well this would have been broken only on 9.3 and there was no production release of 9.3, so I think this is good in my opinion. |
Sebastian if the order changes of an underlying table a view built with Select * can get out of sync. There are two fixes.
Given the prior processes in DNN I used option 2, since that is how we do it. You can recreate this issue easily to confirm. We have also tested on multiple 9.3.0 builds that this resolves |
@mitchelsellers |
@sleupold Yes, looking back at the history, execution of sp_refreshsqlmodule is not something we have done, and if an error occurs we wouldn't have a handle. Thus why I went the other route. I left it as SELECT * for simplicity, if you want to update this PR to list all of the columns, go ahead. But I'm looking to keep the changes as minimal as possible for review etc at this point. I'd like in 9.4, or 10.0 to review & remove all SELECT * usage across the board as a dedicated effort, etc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tested this solution to resolve the issue at hand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving
BTW, to the point of using explicit columns or not: regardless of whether you do this, you need to recreate a view if you change the underlying table unless you're in a really specific use case. In my modules I use the * but use the views to retrieve the "super"-object than inherits from the base table object. Every time I add a column to the table, I need to rerun the view SQL. It's not very hard. When using explcicit columns you need to add the column to the view and also run this change. So I'm not losing any sleep over the approach we take in the future. |
Cool, can someone merge then so we have a working nightly build. |
Given that this already had the needed approvals, from not myself, I merged this change |
Fixes dnnsoftware#2521 by rebuilding the PortalsDefaultLanguage view
* DNN-27517: force user logout after password changed in other place. * DNN-27517: update code by review. * DNN-27517: add host settings to control whether force logout after password changed. * NOJIRA: mark as stable. * Fixed bugs on add/remove user permissions for modules * Change algorithm to SHA1CryptoServiceProvider * Updated Issue Templates to include new RFC template and to support submissions for 9.3.0 release * Corrected structure to avoid issue linking * code review * User registration: end the response after redirect (#2511) * Initial New User Email Not Sending At Time of Creation (#2492) This is alternative way to fix above issue proposed in dnnsoftware/Dnn.AdminExperience#174 As per @sleupold , we need to move email notifications from UI to core part. Once this will be approved and merged, we can remove email notifications from UI and replace it with updated controller method to let notifications to be send to their recipients. fixes #2424 * Fix for missing SQL change (#2522) Fixes #2521 by rebuilding the PortalsDefaultLanguage view * Resolve UserProfile Loading Errors From Unsecure pages (#2494) * NOJIRA: mark as stable. * DNN-21637: add config key. * DNN-26576: prevent same-origin errors when loading popup and iframes from a secure page. * code review * Code review * (DNN-10795) - All pages except home page return 404 (#2032) * DNN-10795 - All pages except home page return 404 I have witnessed that occasionally on app pool recycle all,except the home page, will return 404 until the application pool is recycled a second time. I've reviewed the code & believe that the root cause of the issue is due to the fact that the code that builds the tab index, portalDepths dictionary & tabPaths dictionary is not thread safe. I can see code in the method TabIndexController.FetchTabDictionary is using SharedDictionary classes to store the tab dictionaries, however the code is not thread safe when adding the dictionaries to the cache. Therefore when multiple threads are executing the FetchTabDictionary method it's possible for an empty dictionary to be added to the cache. To resolve this issue the code has been updated so that only one thread can add the dictionaries to cache at a time. * Updated comment to trigger Code Licence workflow. * Added compiled DLLs that include the fix for bug DNN-10795 (All pages except home page return 404) for DNn versions 8.0.4 through 9.2.2 * Recursive read lock acquisitions not allowed (#2423) * DNN-23293 Recursive read lock acquisitions not allowed in this mode. * DNN-23293 Recursive read lock acquisitions not allowed in this mode. * Performance problems when huge number of portal aliases is in use (#2514) * DNN-27498 Performance Issues * DNN-27498 Performance Issues * minor formatting * Fixed case sensitivity issue * Added mixed cased alias support to unit tests * Fixed VanityUrl unit tests * Fixed broken LockStrategy unit tests (#2531) * Delete Fixed-DLLs folder that was added as part of PR for bug DNN-10795. (#2535) * Modules > ModuleCreator > fixed path error (#2527) * Fixed issue in ModuleCreator > Web > template.ascx * Update DNN Platform/Admin Modules/Dnn.Modules.ModuleCreator/Templates/Web/Module - HTML/template.ascx Co-Authored-By: mean2me <emanuele.colonnelli@gmail.com> * All languages are highlighted along with current - add css for languages * Log name of package when uninstalling extensions (#2557) * remove spaces * DNN-20856 After export with Content Localization site language flags disappears from pages (#2578) * Fixed parallel build (#2562) * Set active Nuget package source to All * Fixed parallel build * Inclusion of NDepend logo on the readme. (#2598) * Fix for missing SQL change Fixes #2521 by rebuilding the PortalsDefaultLanguage view * Added attribution to NDepend for the usage of their ADO tooling * Fix image/link markdown * Get language from transferred parameter (#2607) * switch encrypt method. (#2616) * DNN-29484: switch encrypt method. * NuGet Package Improvements Changes to modernize the NuGet packages published by the DNN Platform, fixes #2586. The below-submitted changes in structure have been validated by consultation with the DNN Platform Community, Microsoft Representatives, as well as validation of documentation per the published .nuspec file definition (https://docs.microsoft.com/en-us/nuget/reference/nuspec) In detail, the following items have been changed: * Migration of license information to the suggested <license> node rather than the deprecated <licenseurl> node. * Inclusion of target framework for all included .dll files, this prevents installation of the package to pre-4.5 projects protecting downstream users. * Improved package descriptions based on discussions held in the RFC regarding these improvements * Added Package-to-Package dependencies to ensure quick usage and inclusion * Updated the WebAPI and MVC packages to be holistic packages, including references to ALL needed items to develop using those patterns. All changes are current for DNN Platform version 9.3.0 or later. Packages have been built & tested locally with success. ## Suggested Usage With these improved packages, development & references should be easier. ### MVC Modules `Install-Package DotNetNuke.Web.Mvc` Should be the only needed package installation. It will install all needed dependencies, including the items necessary for WebAPI ### Modules Needing WebAPI (Not MVC) `Install-Package DotNetNuke.WebApi` Should be the only needed package for extensions not using MVC, however, needing to use WebApi for services. This will work well for WebForms or Library projects, etc. that don't need the extra references for MVC/Razor ### WebForms/Limited Modules `Install-Package DotNetNuke.Core` The most simple modules, still using the WebForms pattern can use this package for the smallest footprint For #2600 * Adjust the Source package to include changes from GitVersion (#2609) * remove old ckeditor packaging steps * Remove version to allow GitVersion to set it at build time (#2639) * Adding 09.03.01.SqlDataProvider file * Upgrade DNN to .NET Framework 4.7.2 (#2644) * Upgraded app projects to .NET Framework 4.7.2; Added missing dependency to DotNetNuke.Tests.Core as it was missing DotNetNuke.Web.Client * Removed targetframework web.config reference from Dnn.Modules.Console * Reverted unintended changes
Fixes #2521 by re-building the view for the PortalsDefaultLanguage.